Skip to content

fix(web): collapse side panels by default on mobile#113

Merged
naorsabag merged 2 commits into
masterfrom
fix/mobile-collapse-side-panels
May 10, 2026
Merged

fix(web): collapse side panels by default on mobile#113
naorsabag merged 2 commits into
masterfrom
fix/mobile-collapse-side-panels

Conversation

@naorsabag
Copy link
Copy Markdown
Owner

@naorsabag naorsabag commented May 10, 2026

Both bookmark-tabbed side panels (FLOWS sidebar, INSPECT panel) were starting open everywhere, which on a phone left the canvas pinched between them with almost no room.

New behavior: open by default on viewports ≥ 768px (Tailwind `md` breakpoint), collapsed by default below that. Users can still toggle either panel via its bookmark tab on the canvas edge.

The matchMedia check is pulled into `lib/mobile.ts` so AppFragment.tsx (Pages) and App.tsx (local) stay in sync.

Test plan

  • `npm test -w @openhop/web` (42/42)
  • Visit Pages on a phone: confirm both panels are closed; tapping FLOWS / INSPECT bookmark tabs opens them
  • Visit Pages on desktop: behavior unchanged (both open)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Panels (inspector and sidebar) now start collapsed on mobile and remain open by default on larger viewports for improved mobile usability.
    • Added mobile-viewport detection to drive responsive defaults.
  • Improvements

    • Canvas auto-zoom now better handles pane resizes, restoring focused step framing after layout changes.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 10, 2026

Walkthrough

Adds isMobileViewport() and uses it to default inspector and sidebar closed on mobile; updates FlowCanvas to observe pane resizes, call fitToPane(), clear the focus guard, and re-run focus-based auto-zoom after resize.

Changes

Mobile-Responsive UI & Canvas Resize

Layer / File(s) Summary
Mobile Viewport Detection
packages/web/src/lib/mobile.ts
New exported isMobileViewport() returns true for viewport ≤767px, with safe false fallback when window is unavailable.
Panel Initialization
packages/web/src/App.tsx, packages/web/src/AppFragment.tsx
Both components now import isMobileViewport() and initialize inspectorOpen and sidebarOpen to !isMobileViewport(), leaving inspector side/size unchanged.
FlowCanvas: focus guard
packages/web/src/components/FlowCanvas.tsx
Introduces/relocates lastFocusKeyRef guard preventing repeated setCenter for the same focused step during animations.
FlowCanvas: resize observer
packages/web/src/components/FlowCanvas.tsx
Adds paneResizeTick state and a ResizeObserver that calls fitToPane(), clears lastFocusKeyRef, and increments paneResizeTick on pane resize.
FlowCanvas: auto-zoom deps
packages/web/src/components/FlowCanvas.tsx
Extends the auto-zoom effect dependency list to include paneResizeTick so focused framing re-applies after resizes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: collapsing side panels by default on mobile viewports.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/mobile-collapse-side-panels

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/web/src/components/FlowCanvas.tsx (1)

346-352: ⚡ Quick win

Honor prefers-reduced-motion for camera transitions.

These 500ms setCenter tweens now run on every focus/overview change with no reduced-motion fallback. Please switch duration to 0 when the user has prefers-reduced-motion: reduce enabled.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/web/src/components/FlowCanvas.tsx` around lines 346 - 352, The
moveTo function currently always passes duration: 500 to reactFlow.setCenter;
change it to respect the user's prefers-reduced-motion setting by querying
window.matchMedia('(prefers-reduced-motion: reduce)').matches and use duration:
0 when true (otherwise 500). Update the call in moveTo (the reactFlow.setCenter
invocation) to compute a duration variable based on that media query so camera
transitions are disabled for users who prefer reduced motion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/web/src/components/FlowCanvas.tsx`:
- Around line 325-381: The effect currently skips re-applying the camera when
the pane is resized because focusKey equals lastFocusKeyRef.current; fix by
detecting pane size changes and forcing a re-focus: read
pane.offsetWidth/offsetHeight into a local pair (or store in a new
lastPaneSizeRef), and if those dimensions differ from the previous stored ones,
reset lastFocusKeyRef.current (or update it) so moveTo(...) runs again; modify
the useEffect to compare/update pane size (using containerRef -> '.react-flow'
offsetWidth/offsetHeight) and reference lastFocusKeyRef, moveTo, computeBbox,
and reactFlow so the camera is reapplied on resize during playback.

---

Nitpick comments:
In `@packages/web/src/components/FlowCanvas.tsx`:
- Around line 346-352: The moveTo function currently always passes duration: 500
to reactFlow.setCenter; change it to respect the user's prefers-reduced-motion
setting by querying window.matchMedia('(prefers-reduced-motion:
reduce)').matches and use duration: 0 when true (otherwise 500). Update the call
in moveTo (the reactFlow.setCenter invocation) to compute a duration variable
based on that media query so camera transitions are disabled for users who
prefer reduced motion.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5b2f933f-5771-4bdf-86fe-de006eb538b4

📥 Commits

Reviewing files that changed from the base of the PR and between ea58e1a and 9b3583c.

📒 Files selected for processing (5)
  • packages/web/src/App.tsx
  • packages/web/src/AppFragment.tsx
  • packages/web/src/components/FlowCanvas.tsx
  • packages/web/src/lib/mobile.ts
  • packages/web/vitest.config.ts

Comment thread packages/web/src/components/FlowCanvas.tsx
Both bookmark-tabbed side panels were starting open everywhere,
which on a phone left the canvas pinched between them with almost
no room. New behavior: open by default on viewports >= 768px (the
Tailwind `md` breakpoint), collapsed by default below that. Users
can still toggle either panel via its bookmark tab.

Pulled the matchMedia check into lib/mobile.ts so AppFragment.tsx
(Pages) and App.tsx (local) stay in sync.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@naorsabag naorsabag force-pushed the fix/mobile-collapse-side-panels branch from 9b3583c to 7b6240e Compare May 10, 2026 16:58
…o overview

When the user toggles FLOWS / INSPECT during playback, the
ResizeObserver fires fitToPane(), which centers the full diagram and
overrides the auto-zoom's setCenter. The auto-zoom effect then
no-ops because focusKey hasn't changed — so playback visibly drops
back to the overview until the next step advance.

Have the ResizeObserver, in addition to fitToPane():
  - clear lastFocusKeyRef.current so the focusKey guard doesn't skip
  - bump a paneResizeTick state, which is a dep of the auto-zoom
    effect, so React re-runs it

Together those make resize-triggered fitToPane() not the last word
during playback — the auto-zoom effect re-locks onto the active
step's nodes immediately after.

(Caught by CodeRabbit on #113.)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/web/src/components/FlowCanvas.tsx (1)

202-210: ⚡ Quick win

Throttle resize-driven viewport updates to one frame.

During panel width transitions, ResizeObserver can fire many times in quick succession; running fitToPane() + setPaneResizeTick() each callback can cause avoidable camera churn.

Proposed patch
   useEffect(() => {
     const pane = containerRef.current?.querySelector('.react-flow') as HTMLElement | null
     if (!pane || typeof ResizeObserver === 'undefined') return
+    let rafId: number | null = null
+    let pending = false
     const observer = new ResizeObserver(() => {
-      fitToPane()
-      // Force the auto-zoom effect to re-apply: clear the focus guard
-      // and bump the tick that the effect depends on. Together they
-      // make resize-triggered fitToPane() not the last word during
-      // playback.
-      lastFocusKeyRef.current = ''
-      setPaneResizeTick((t) => t + 1)
+      if (pending) return
+      pending = true
+      rafId = window.requestAnimationFrame(() => {
+        pending = false
+        fitToPane()
+        // Force the auto-zoom effect to re-apply: clear the focus guard
+        // and bump the tick that the effect depends on.
+        lastFocusKeyRef.current = ''
+        setPaneResizeTick((t) => t + 1)
+      })
     })
     observer.observe(pane)
-    return () => observer.disconnect()
+    return () => {
+      observer.disconnect()
+      if (rafId !== null) window.cancelAnimationFrame(rafId)
+    }
   }, [fitToPane])
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/web/src/components/FlowCanvas.tsx` around lines 202 - 210, Throttle
the ResizeObserver callback to at most one animation frame: instead of calling
fitToPane(), clearing lastFocusKeyRef.current and setPaneResizeTick() on every
ResizeObserver event, schedule a single requestAnimationFrame (store its id in a
local/ref variable) and only run the three actions inside that raf handler,
ignoring additional ResizeObserver callbacks until the scheduled frame runs (and
cancel any existing raf when scheduling a new one if desired). Update the
ResizeObserver creation code that currently references fitToPane,
lastFocusKeyRef, and setPaneResizeTick to use this raf-based throttle and ensure
you clear the raf on cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/web/src/components/FlowCanvas.tsx`:
- Around line 202-210: Throttle the ResizeObserver callback to at most one
animation frame: instead of calling fitToPane(), clearing
lastFocusKeyRef.current and setPaneResizeTick() on every ResizeObserver event,
schedule a single requestAnimationFrame (store its id in a local/ref variable)
and only run the three actions inside that raf handler, ignoring additional
ResizeObserver callbacks until the scheduled frame runs (and cancel any existing
raf when scheduling a new one if desired). Update the ResizeObserver creation
code that currently references fitToPane, lastFocusKeyRef, and setPaneResizeTick
to use this raf-based throttle and ensure you clear the raf on cleanup.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 72a5eeee-5145-4ee7-85b3-d4e2bbad7dd2

📥 Commits

Reviewing files that changed from the base of the PR and between 7b6240e and 80a8128.

📒 Files selected for processing (1)
  • packages/web/src/components/FlowCanvas.tsx

@naorsabag naorsabag merged commit 25d79ff into master May 10, 2026
8 checks passed
@naorsabag naorsabag deleted the fix/mobile-collapse-side-panels branch May 10, 2026 17:38
naorsabag added a commit that referenced this pull request May 10, 2026
…test

- @openhop/server: 0.1.0-beta.2 → 0.1.0
- @openhop/web:    0.1.0-beta.4 → 0.1.0
- openhop CLI:     0.1.0-beta.6 → 0.1.0
  + bump pinned @openhop/server, @openhop/web deps + hardcoded --version

publish.yml: drop `--tag beta` from all three publish steps. With no
--tag, `npm publish` sets the `latest` dist-tag — so npmjs.com pages
and `npm install <pkg>` (no @beta suffix) automatically reflect the
new version on every successful publish. No more manual
`npm dist-tag add latest` step.

Bundles 13 PRs since v0.1.0-beta.6:

Web (heavy):
- #106 sprite URLs use Vite BASE_URL (Pages 404 fix)
- #107 example flow cards on empty state
- #108 sidebar + carrot-click highlight for string-data steps
- #109 all 5 examples grouped under examples/ + first-visit autoload
- #112 per-step auto-zoom + playback speed button
- #113 collapse FLOWS / INSPECT by default on mobile
- #114 lazy editor + vendor-split chunks + meta description
- #115 filter codemirror from <modulepreload>
- #117 memo FlowNode + GPU-layer sprites for smoother pan/zoom

CLI:
- #116 OpenSkills fallback hint when no Tier-1 client gets the skill
- #118 suppress the hint when Tier-1 already has the skill

Server: untouched code; bumped to keep the 0.1.0 set consistent.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
naorsabag added a commit that referenced this pull request May 11, 2026
…test (#119)

- @openhop/server: 0.1.0-beta.2 → 0.1.0
- @openhop/web:    0.1.0-beta.4 → 0.1.0
- openhop CLI:     0.1.0-beta.6 → 0.1.0
  + bump pinned @openhop/server, @openhop/web deps + hardcoded --version

publish.yml: drop `--tag beta` from all three publish steps. With no
--tag, `npm publish` sets the `latest` dist-tag — so npmjs.com pages
and `npm install <pkg>` (no @beta suffix) automatically reflect the
new version on every successful publish. No more manual
`npm dist-tag add latest` step.

Bundles 13 PRs since v0.1.0-beta.6:

Web (heavy):
- #106 sprite URLs use Vite BASE_URL (Pages 404 fix)
- #107 example flow cards on empty state
- #108 sidebar + carrot-click highlight for string-data steps
- #109 all 5 examples grouped under examples/ + first-visit autoload
- #112 per-step auto-zoom + playback speed button
- #113 collapse FLOWS / INSPECT by default on mobile
- #114 lazy editor + vendor-split chunks + meta description
- #115 filter codemirror from <modulepreload>
- #117 memo FlowNode + GPU-layer sprites for smoother pan/zoom

CLI:
- #116 OpenSkills fallback hint when no Tier-1 client gets the skill
- #118 suppress the hint when Tier-1 already has the skill

Server: untouched code; bumped to keep the 0.1.0 set consistent.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant